QPACK security considerations #3575

afrind · 2020-04-13T21:59:45Z

Behold

Fixes #1737

Behold Fixes: 1737

martinthomson

This looks good. But then I'm not impartial in this.

An informative reference to HPACK might be wise. Otherwise people might get an unexplained sense of déjà vu.

martinthomson · 2020-04-14T01:03:20Z

draft-ietf-quic-qpack.md

+
+Note:
+
+Padding schemes only provide limited protection against an attacker with these


Suggested change

Padding schemes only provide limited protection against an attacker with these

: Padding schemes only provide limited protection against an attacker with these

martinthomson · 2020-04-14T01:06:40Z

draft-ietf-quic-qpack.md

+table are attributed to an entity, and only the entity that created a particular
+value can extract that value.
+
+To improve compression performance of tqhis option, certain entries might be tagged as being public. For example, a web browser might make the values of the Accept-Encoding header field available in all requests.


Suggested change

To improve compression performance of tqhis option, certain entries might be tagged as being public. For example, a web browser might make the values of the Accept-Encoding header field available in all requests.

To improve compression performance of this option, certain entries might be

tagged as being public. For example, a web browser might make the values of the

Accept-Encoding header field available in all requests.

martinthomson · 2020-04-14T01:07:03Z

draft-ietf-quic-qpack.md

+
+Note:
+
+Simply removing entries corresponding to the header field from the dynamic table


Suggested change

Simply removing entries corresponding to the header field from the dynamic table

: Simply removing entries corresponding to the header field from the dynamic table

martinthomson · 2020-04-14T01:07:59Z

draft-ietf-quic-qpack.md

+that a large number of attempts to guess a header field value results in the
+header field no more being compared to the dynamic table entries in future
+messages, effectively preventing further guesses.


Suggested change

that a large number of attempts to guess a header field value results in the

header field no more being compared to the dynamic table entries in future

messages, effectively preventing further guesses.

that a large number of attempts to guess a header field value results in the

header field not being compared to the dynamic table entries in future

messages, effectively preventing further guesses.

I had a little trouble with "no more".

martinthomson · 2020-04-14T01:09:02Z

draft-ietf-quic-qpack.md

+field value. Marking a header field as not using the dynamic table any more
+might occur for shorter values more quickly or with higher probability than for
+longer values.


Suggested change

field value. Marking a header field as not using the dynamic table any more

might occur for shorter values more quickly or with higher probability than for

longer values.

field value. Disabling access to the dynamic table for a header field might

occur for shorter values more quickly or with higher probability than for longer

values.

martinthomson · 2020-04-14T01:10:04Z

draft-ietf-quic-qpack.md

+Note that these criteria for deciding to use a never indexed literal
+representation will evolve over time as new attacks are discovered.
+
+##. Static Huffman Encoding


Suggested change

##. Static Huffman Encoding

## Static Huffman Encoding

draft-ietf-quic-qpack.md

martinthomson · 2020-04-14T01:11:06Z

draft-ietf-quic-qpack.md

+designed to limit both the peak and state amounts of memory allocated by an
+endpoint.


Suggested change

designed to limit both the peak and state amounts of memory allocated by an

endpoint.

designed to limit both the peak and stable amounts of memory allocated by an

endpoint.

This error is in the HPACK RFC. Is it worth an errata?

I'll let you manage that :) You can be the author of an RFC erratum. The glory!

martinthomson · 2020-04-14T01:11:26Z

draft-ietf-quic-qpack.md

+QPACK through the definition of the maximum size of the dynamic table, and the
+maximum number of blocking streams. In HTTP/3, these values are controlled by
+the decoder through the setting parameter QPACK_MAX_TABLE_CAPACITY and
+QPACK_BLOCKED_STREAMS, respectively (see Section


Suggested change

QPACK_BLOCKED_STREAMS, respectively (see Section

QPACK_BLOCKED_STREAMS, respectively (see

martinthomson · 2020-04-14T01:13:47Z

draft-ietf-quic-qpack.md

+HTTP/3, this is realized by setting an appropriate value for the
+QPACK_MAX_TABLE_CAPACITY parameter. An encoder can limit the amount of state
+memory it uses by signaling lower dynamic table size than the decoder allows
+(see {{eviction}}).  A decoder can limit the amount of state memory used for


Suggested change

(see {{eviction}}). A decoder can limit the amount of state memory used for

(see {{eviction}}).

A decoder can limit the amount of state memory used for

LPardue

lgtm modulo some nits

LPardue · 2020-04-14T22:14:47Z

draft-ietf-quic-qpack.md

+recovered successfully. However, values with low entropy remain vulnerable.
+
+Attacks of this nature are possible any time that two mutually distrustful
+entities control requests or responses that are placed onto a single HTTP/2


Suggested change

entities control requests or responses that are placed onto a single HTTP/2

entities control requests or responses that are placed onto a single HTTP/3

LPardue · 2020-04-14T22:28:02Z

draft-ietf-quic-qpack.md

+A decoder can limit the amount of state memory used for the dynamic table by
+setting an appropriate value for the maximum size of the dynamic table. In
+HTTP/3, this is realized by setting an appropriate value for the
+QPACK_MAX_TABLE_CAPACITY parameter. An encoder can limit the amount of state


nit: the doc is inconsistent about QPACK_MAX_TABLE_CAPACITY vs SETTINGS_QPACK_MAX_TABLE_CAPACITY. I don't know what the answer is

The text seems to consistently use SETTINGS_ variants, but the table omits it. I'll fix these to match the text, but SETTINGS_ seems redundant in the table. I'll resolve the inconsistency in another PR.

It's an (unstated) norm to omit SETTINGS_ in cases where it's already blatantly obvious that we're discussing a setting. You'll find similar variance in 7540, 7541, and HTTP/3.

LPardue · 2020-04-14T22:28:48Z

draft-ietf-quic-qpack.md

+setting an appropriate value for the maximum size of the dynamic table. In
+HTTP/3, this is realized by setting an appropriate value for the
+QPACK_MAX_TABLE_CAPACITY parameter. An encoder can limit the amount of state
+memory it uses by signaling lower dynamic table size than the decoder allows


Suggested change

memory it uses by signaling lower dynamic table size than the decoder allows

memory it uses by signaling a lower dynamic table size than the decoder allows

LPardue · 2020-04-14T22:29:52Z

draft-ietf-quic-qpack.md

+(see {{eviction}}).  A decoder can limit the amount of state memory used for
+blocked streams by setting an appropriate value for the maximum number of
+blocked streams.  In HTTP/3, this is realized by setting an appropriate value
+for the QPACK_BLOCKED_STREAMS parameter.  An encoder can limit the amount of


nit: QPACK_BLOCKED_STREAMS vs. SETTINGS_QPACK_BLOCKED_STREAMS

MikeBishop

All looks good (and familiar); some minor nits.

draft-ietf-quic-qpack.md

MikeBishop · 2020-04-16T20:26:30Z

draft-ietf-quic-qpack.md

+An implementation has to set a limit for the values it accepts for integers, as
+well as for the encoded length (see {{prefixed-integers}}). In the same way, it
+has to set a limit to the length it accepts for string literals (see
+{{string-literals}}).


I believe we have a minimum size which MUST be supported; should that be mentioned here?

@MikeBishop : I can't quite figure an elegant way to mention the minimum here. It is already mentioned in the referenced section.

bencebeky · 2020-04-22T13:33:55Z

draft-ietf-quic-qpack.md

+SETTINGS_QPACK_BLOCKED_STREAMS, respectively (see
+{{maximum-dynamic-table-capacity}} and {{blocked-streams}}). The limit on the
+size of the dynamic table takes into account both the size of the data stored in
+the dynamic table, plus a small allowance for overhead.  The limit on the number


"both" does not flow well with "plus". Either s/", plus"/" and"/, or remove "both".

LPardue · 2020-04-28T16:14:24Z

@afrind I think this is inches of getting landed

Feedback from Mike Co-authored-by: Mike Bishop <mbishop@evequefou.be>

QPACK security considerations

25f837c

Behold Fixes: 1737

ianswett requested a review from bencebeky April 13, 2020 22:15

martinthomson approved these changes Apr 14, 2020

View reviewed changes

LPardue approved these changes Apr 14, 2020

View reviewed changes

afrind added 2 commits April 14, 2020 15:31

Fix lint

aa8fed6

Feedback from Martin and Lucas

4c3256a

martinthomson added -qpack editorial An issue that does not affect the design of the protocol; does not require consensus. labels Apr 15, 2020

MikeBishop approved these changes Apr 16, 2020

View reviewed changes

bencebeky approved these changes Apr 22, 2020

View reviewed changes

afrind and others added 2 commits May 5, 2020 15:42

Apply suggestions from code review

090e722

Feedback from Mike Co-authored-by: Mike Bishop <mbishop@evequefou.be>

Wordsmithing suggestion from Bence

753a49d

afrind merged commit dcf603b into master May 6, 2020

afrind deleted the qpack-security branch May 6, 2020 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QPACK security considerations #3575

QPACK security considerations #3575

afrind commented Apr 13, 2020 •

edited by MikeBishop

martinthomson left a comment

martinthomson Apr 14, 2020

martinthomson Apr 14, 2020

martinthomson Apr 14, 2020

martinthomson Apr 14, 2020

martinthomson Apr 14, 2020

martinthomson Apr 14, 2020

martinthomson Apr 14, 2020

afrind Apr 14, 2020

martinthomson Apr 15, 2020

martinthomson Apr 14, 2020

martinthomson Apr 14, 2020

LPardue left a comment

LPardue Apr 14, 2020

LPardue Apr 14, 2020

afrind Apr 14, 2020

LPardue Apr 14, 2020

MikeBishop Apr 15, 2020

LPardue Apr 14, 2020

LPardue Apr 14, 2020

MikeBishop left a comment

MikeBishop Apr 16, 2020

afrind May 5, 2020

bencebeky Apr 22, 2020

LPardue commented Apr 28, 2020


		Note:

		Padding schemes only provide limited protection against an attacker with these

	Padding schemes only provide limited protection against an attacker with these
	: Padding schemes only provide limited protection against an attacker with these


		Note:

		Simply removing entries corresponding to the header field from the dynamic table

	Simply removing entries corresponding to the header field from the dynamic table
	: Simply removing entries corresponding to the header field from the dynamic table

		designed to limit both the peak and state amounts of memory allocated by an
		endpoint.

	QPACK_BLOCKED_STREAMS, respectively (see Section
	QPACK_BLOCKED_STREAMS, respectively (see

	entities control requests or responses that are placed onto a single HTTP/2
	entities control requests or responses that are placed onto a single HTTP/3

	memory it uses by signaling lower dynamic table size than the decoder allows
	memory it uses by signaling a lower dynamic table size than the decoder allows

QPACK security considerations #3575

QPACK security considerations #3575

Conversation

afrind commented Apr 13, 2020 • edited by MikeBishop

martinthomson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LPardue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeBishop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LPardue commented Apr 28, 2020

afrind commented Apr 13, 2020 •

edited by MikeBishop